tests/llama_stack/constants.py (2)

17-17: Constant addition looks good; confirm selector correctness and uniqueness

Please verify this label key/value exactly matches the operator Pod labels in your cluster/manifests and that it uniquely identifies a single Pod (rollouts can briefly create >1). If there’s a more specific label available (e.g., component/controller), prefer that to reduce flakiness.

I can provide a quick check script if helpful.

---

17-17: Prefer “selector” over “filter” for k8s terminology

To align with Kubernetes terminology and the label_selector param used elsewhere, consider renaming to LLS_OPERATOR_POD_SELECTOR.

Apply in this file:

-LLS_OPERATOR_POD_FILTER: str = "app.kubernetes.io/name=llama-stack-k8s-operator"
+LLS_OPERATOR_POD_SELECTOR: str = "app.kubernetes.io/name=llama-stack-k8s-operator"

…and update imports/usages in the related fixture (see comments there).

tests/llama_stack/image_validation/conftest.py (3)

13-20: Silence Ruff ARG001 and simplify fixture (return vs yield)

The model_namespace arg is intentionally unused for ordering, but Ruff flags it. Also, since there’s no teardown, return is clearer than yield.

Apply:

-@pytest.fixture(scope="class")
-def lls_operator_pod_by_label(admin_client: DynamicClient, model_namespace) -> Generator[Pod, Any, Any]:
+@pytest.fixture(scope="class")
+def lls_operator_pod_by_label(admin_client: DynamicClient, _model_namespace) -> Generator[Pod, Any, Any]:
     """Get the LLS operator pod."""
-    yield wait_for_pods_by_labels(
+    return wait_for_pods_by_labels(
         admin_client=admin_client,
         namespace=py_config["applications_namespace"],
-        label_selector=LLS_OPERATOR_POD_FILTER,
+        label_selector=LLS_OPERATOR_POD_FILTER,
         expected_num_pods=1,
     )[0]

---

8-8: Follow-up if you adopt the “selector” rename

If you rename the constant per the other comment, update the import accordingly.

-from tests.llama_stack.constants import LLS_OPERATOR_POD_FILTER
+from tests.llama_stack.constants import LLS_OPERATOR_POD_SELECTOR

…and the call site:

-        label_selector=LLS_OPERATOR_POD_FILTER,
+        label_selector=LLS_OPERATOR_POD_SELECTOR,

---

15-20: Brittleness: expecting exactly one Pod can flake during rollouts

expected_num_pods=1 will fail if the controller briefly runs two Pods (e.g., during upgrades). If a more specific label is available, tighten the selector; otherwise, consider handling > 1 by choosing the Ready Pod.

I can propose a helper that picks the Ready Pod when multiple are returned.

tests/llama_stack/image_validation/test_verify_lls_rhoai_images.py (5)

38-38: Remove unused admin_client parameter

Not used in the test; drop it to silence linters and clarify dependencies.

     def test_verify_lls_operator_images(
-        self: Self,
-        admin_client: DynamicClient,
+        self: Self,
         lls_operator_pod_by_label: Pod,
         related_images_refs: Set[str],
     ):

Additionally, remove the now-unused import:

-from kubernetes.dynamic import DynamicClient

---

42-50: Simplify: no need to wrap a single Pod in a list

Call the validator once and fail on errors directly.

-        validation_errors = []
-        for pod in [lls_operator_pod_by_label]:
-            validation_errors.extend(
-                validate_container_images(
-                    pod=pod, valid_image_refs=related_images_refs, skip_patterns=["openshift-service-mesh"]
-                )
-            )
-
-        if validation_errors:
-            pytest.fail("\n".join(validation_errors))
+        errors = validate_container_images(
+            pod=lls_operator_pod_by_label,
+            valid_image_refs=related_images_refs,
+            skip_patterns=["openshift-service-mesh"],
+        )
+        if errors:
+            pytest.fail("\n".join(errors))

---

1-3: Drop unused logger

LOGGER isn’t used; remove the import and assignment to reduce noise.

-from simple_logger.logger import get_logger
@@
-LOGGER = get_logger(name=__name__)

Also applies to: 12-12

---

16-24: Do we need model_namespace parametrization here?

The operator Pod is looked up in py_config["applications_namespace"], not in the parametrized model_namespace. If this namespace isn’t required for LLS operator validation, consider removing the parametrization to speed up tests and reduce setup.

---

2-2: Optional: avoid typing.Self for broader Python compatibility

Unless you’re pinned to 3.11+, consider dropping Self to avoid version coupling.

-from typing import Self, Set
+from typing import Set
@@
-        self: Self,
+        self,

If you prefer modern types and are on 3.11+, this is fine to keep.

Also applies to: 37-37

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Learnt from: adolfo-ab
PR: opendatahub-io/opendatahub-tests#334
File: tests/model_explainability/trustyai_service/test_trustyai_service.py:52-65
Timestamp: 2025-06-05T10:05:17.642Z
Learning: For TrustyAI image validation tests: operator image tests require admin_client, related_images_refs, and trustyai_operator_configmap fixtures, while service image tests would require different fixtures like trustyai_service_with_pvc_storage, model_namespace, and current_client_token.

Learnt from: adolfo-ab
PR: opendatahub-io/opendatahub-tests#334
File: tests/model_explainability/trustyai_service/test_trustyai_service.py:52-65
Timestamp: 2025-06-05T10:05:17.642Z
Learning: For TrustyAI image validation tests: operator image tests require admin_client, related_images_refs, and trustyai_operator_configmap fixtures, while service image tests would require different fixtures like trustyai_service_with_pvc_storage, model_namespace, and current_client_token.

tests/llama_stack/image_validation/conftest.py (1)

11-19: Singular naming and direct yield (optional).

The fixture yields a single Pod; rename to singular to avoid confusion and drop list indexing at call sites.

Apply this diff here:

-@pytest.fixture(scope="class")
-def lls_pods(admin_client: DynamicClient, model_namespace) -> Generator[Pod, Any, Any]:
+@pytest.fixture(scope="class")
+def lls_operator_pod(admin_client: DynamicClient, model_namespace) -> Generator[Pod, Any, Any]:
@@
-    yield wait_for_pods_by_labels(
+    yield wait_for_pods_by_labels(
@@
-    )[0]
+    )[0]

And update test(s) to use lls_operator_pod.

tests/llama_stack/image_validation/test_verify_lls_rhoai_images.py (4)

3-3: Remove unused LOGGER.

Not used; keep the module lean.

-from simple_logger.logger import get_logger
@@
-LOGGER = get_logger(name=__name__)

Also applies to: 12-12

---

4-4: Drop unused admin_client param (ruff ARG002).

Signature can be simplified; DynamicClient import becomes unnecessary.

-from kubernetes.dynamic import DynamicClient
@@
-    def test_verify_lls_operator_images(
-        self: Self,
-        admin_client: DynamicClient,
-        lls_pods: Pod,
-        related_images_refs: Set[str],
-    ):
+    def test_verify_lls_operator_images(
+        self: Self,
+        lls_pods: Pod,
+        related_images_refs: Set[str],
+    ):

If you accept the singular fixture rename, also switch lls_pods → lls_operator_pod.

Also applies to: 36-41

---

2-2: Prefer built‑in set annotations; remove typing.Set import.

Modern Python supports set[str].

-from typing import Self, Set
+from typing import Self
@@
-        related_images_refs: Set[str],
+        related_images_refs: set[str],

Also applies to: 40-41

---

42-49: Simplify iteration over a single pod.

No need to build a one‑element list.

-        validation_errors = []
-        for pod in [lls_pods]:
-            validation_errors.extend(
-                validate_container_images(
-                    pod=pod, valid_image_refs=related_images_refs, skip_patterns=["openshift-service-mesh"]
-                )
-            )
+        validation_errors = validate_container_images(
+            pod=lls_pods,
+            valid_image_refs=related_images_refs,
+            skip_patterns=["openshift-service-mesh"],
+        )

If you adopt the singular fixture rename, replace lls_pods with lls_operator_pod.

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Learnt from: adolfo-ab
PR: opendatahub-io/opendatahub-tests#334
File: tests/model_explainability/trustyai_service/test_trustyai_service.py:52-65
Timestamp: 2025-06-05T10:05:17.642Z
Learning: For TrustyAI image validation tests: operator image tests require admin_client, related_images_refs, and trustyai_operator_configmap fixtures, while service image tests would require different fixtures like trustyai_service_with_pvc_storage, model_namespace, and current_client_token.

tests/llama_stack/image_validation/test_verify_lls_rhoai_images.py (5)

27-32: Align naming: operator vs core (Distribution) pod

The test name says “operator” but the fixture/doc for lls_pods (and LLS_CORE_POD_FILTER) targets the core/Distribution pod. Align to avoid confusion.

Apply this diff to reflect core pod validation:

 class TestLLSImages:
     """
-    Tests to verify that LLS (LlamaStack) Distribution image meets the requirements:
+    Tests to verify that LLS (LlamaStack) core (Distribution) pod images meet the requirements:
     1. Images are hosted in registry.redhat.io
     2. Images use sha256 digest instead of tags
     3. Images are listed in the CSV's relatedImages section
     """
 
     @pytest.mark.smoke
-    def test_verify_lls_operator_images(
+    def test_verify_lls_core_images(

If you intended to check the operator Pod instead, please update the label selector used by the fixture to target the operator and keep the original test name.

Also applies to: 35-35

---

6-7: Remove unused logger

LOGGER is never used here.

-from simple_logger.logger import get_logger
-
-
-LOGGER = get_logger(name=__name__)
+

Also applies to: 11-11

---

1-2: Drop unused/over-specific typing and unused fixture arg

• admin_client isn’t used (Ruff ARG002); remove it.
• Avoid annotating self.
• Prefer built-in generics (set[str]) and remove unused imports.

-from typing import Self, Set
-
-from kubernetes.dynamic import DynamicClient
+


@@
-    def test_verify_lls_operator_images(
-        self: Self,
-        admin_client: DynamicClient,
-        lls_pods: Pod,
-        related_images_refs: Set[str],
-    ):
+    def test_verify_lls_core_images(
+        self,
+        lls_pods: Pod,
+        related_images_refs: set[str],
+    ):

Also applies to: 3-3, 35-40

---

38-38: Nit: fixture name suggests plurality but returns a single Pod

lls_pods returns a single Pod (index [0] in the fixture). Consider renaming the fixture to lls_pod for clarity.

---

41-44: Replace single-item loop with direct call and skip service-mesh sidecars

validate_container_images already accepts skip_patterns — simplify the loop and skip known service-mesh sidecars.

-        validation_errors = []
-        for pod in [lls_pods]:
-            validation_errors.extend(validate_container_images(pod=pod, valid_image_refs=related_images_refs))
+        validation_errors = validate_container_images(
+            pod=lls_pods,
+            valid_image_refs=related_images_refs,
+            skip_patterns=["openshift-service-mesh"],
+        )

If your environment uses different sidecar names (e.g., Istio’s proxyv2), extend skip_patterns accordingly.

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Learnt from: lugi0
PR: opendatahub-io/opendatahub-tests#487
File: tests/model_registry/async_job/test_async_upload_e2e.py:344-356
Timestamp: 2025-08-08T15:59:16.170Z
Learning: In opendatahub-io/opendatahub-tests, tests/model_registry/async_job/test_async_upload_e2e.py’s test_end_to_end_async_upload_job intentionally expects an OCI image index (application/vnd.oci.image.index.v1+json) from pull_manifest_from_oci_registry and asserts on the "manifests" array. This format has been verified by the author (lugi0); no change to Accept headers or assertions is required.

Learnt from: adolfo-ab
PR: opendatahub-io/opendatahub-tests#334
File: tests/model_explainability/trustyai_service/test_trustyai_service.py:52-65
Timestamp: 2025-06-05T10:05:17.642Z
Learning: For TrustyAI image validation tests: operator image tests require admin_client, related_images_refs, and trustyai_operator_configmap fixtures, while service image tests would require different fixtures like trustyai_service_with_pvc_storage, model_namespace, and current_client_token.

Learnt from: adolfo-ab
PR: opendatahub-io/opendatahub-tests#334
File: tests/model_explainability/trustyai_service/test_trustyai_service.py:52-65
Timestamp: 2025-06-05T10:05:17.642Z
Learning: For TrustyAI image validation tests: operator image tests require admin_client, related_images_refs, and trustyai_operator_configmap fixtures, while service image tests would require different fixtures like trustyai_service_with_pvc_storage, model_namespace, and current_client_token.